Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[espmilighthub] Initial contribution #9218

Merged
merged 41 commits into from
Jan 29, 2021
Merged

[espmilighthub] Initial contribution #9218

merged 41 commits into from
Jan 29, 2021

Conversation

Skinah
Copy link
Contributor

@Skinah Skinah commented Dec 4, 2020 •
edited
Loading

A binding that allows an esp8266 based opensource hub to bridge openHAB to Milight globes.

Binding is used by more than 50 users as it has been around since Feb 2018 but I only just updated it to now no longer use the Paho lib and instead use the openHAB MQTT transport. Forum thread on this binding is found here:
https://community.openhab.org/t/espmilighthub-new-binding-for-milight-limitlessled-and-easybulb/39720

The hardware hub has a github project here:
https://github.com/sidoh/esp8266_milight_hub/releases101

A precompiled JAR for V3.0 can be downloaded here:
https://openhab.jfrog.io/artifactory/libs-pullrequest-local/org/openhab/addons/bundles/org.openhab.binding.mqtt.espmilighthub/3.1.0-SNAPSHOT/org.openhab.binding.mqtt.espmilighthub-3.1.0-SNAPSHOT.jar

Signed-off-by: Matthew Skinner matt@pcmus.com

@Skinah Skinah requested a review from a team as a code owner December 4, 2020 04:14
@Skinah Skinah added the new binding If someone has started to work on a binding. For a new binding PR. label Dec 4, 2020
@fwolter fwolter self-assigned this Dec 8, 2020
fwolter
fwolter requested changes Dec 8, 2020
View reviewed changes
bundles/org.openhab.binding.mqtt.espmilighthub/README.md Outdated Show resolved Hide resolved
bundles/org.openhab.binding.mqtt.espmilighthub/README.md Outdated Show resolved Hide resolved
bundles/org.openhab.binding.mqtt.espmilighthub/README.md Outdated Show resolved Hide resolved
bundles/org.openhab.binding.mqtt.espmilighthub/README.md Outdated Show resolved Hide resolved
bundles/org.openhab.binding.mqtt.espmilighthub/README.md Show resolved Hide resolved
.../main/java/org/openhab/binding/mqtt/espmilighthub/internal/handler/EspMilightHubHandler.java Outdated Show resolved Hide resolved
bundles/org.openhab.binding.mqtt.espmilighthub/src/main/resources/OH-INF/thing/thing-types.xml Outdated Show resolved Hide resolved
bundles/org.openhab.binding.mqtt.espmilighthub/src/main/resources/OH-INF/thing/thing-types.xml Outdated Show resolved Hide resolved
bundles/org.openhab.binding.mqtt.espmilighthub/src/main/resources/OH-INF/thing/thing-types.xml Outdated Show resolved Hide resolved
bundles/org.openhab.binding.mqtt.espmilighthub/src/main/resources/OH-INF/thing/thing-types.xml Outdated Show resolved Hide resolved
@fwolter
Copy link
Member

fwolter commented Dec 19, 2020

Please raise your hand if you finished making all changes.

@Skinah
Copy link
Contributor Author

Skinah commented Dec 20, 2020

Please raise your hand if you finished making all changes.

Yes finished all changes, dont know how to raise hand and google did not return any hits on how to do that so will re-request the review from you as a way to flag this. Thanks for the review when things must be very busy.

@Skinah Skinah requested a review from fwolter December 20, 2020 00:32
fwolter
fwolter requested changes Dec 25, 2020
View reviewed changes
bundles/org.openhab.binding.mqtt.espmilighthub/pom.xml Outdated Show resolved Hide resolved
bundles/org.openhab.binding.mqtt.espmilighthub/src/main/resources/OH-INF/config/config.xml Outdated Show resolved Hide resolved
bundles/org.openhab.binding.mqtt.espmilighthub/src/main/resources/OH-INF/config/config.xml Outdated Show resolved Hide resolved
bundles/org.openhab.binding.mqtt.espmilighthub/src/main/resources/OH-INF/config/config.xml Outdated Show resolved Hide resolved
bundles/org.openhab.binding.mqtt.espmilighthub/src/main/resources/OH-INF/config/config.xml Outdated Show resolved Hide resolved
bundles/org.openhab.binding.mqtt.espmilighthub/src/main/resources/OH-INF/config/config.xml Outdated Show resolved Hide resolved
bundles/org.openhab.binding.mqtt.espmilighthub/src/main/resources/OH-INF/config/config.xml Outdated Show resolved Hide resolved
@Skinah Skinah requested a review from fwolter December 29, 2020 03:52
@Skinah
Copy link
Contributor Author

Skinah commented Dec 30, 2020

RAISE HAND. All changes requested are addressed.

fwolter
fwolter approved these changes Dec 30, 2020
View reviewed changes
Copy link
Member

@fwolter fwolter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@fwolter fwolter added the cre Coordinated Review Effort label Dec 31, 2020
cpmeister
cpmeister requested changes Jan 18, 2021
View reviewed changes
Copy link
Contributor

@cpmeister cpmeister left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You also need to add this binding to the footer.xml file otherwise it won't get included in the mqtt binding.

...org/openhab/binding/mqtt/espmilighthub/internal/discovery/EspMilightHubDiscoveryService.java Outdated Show resolved Hide resolved
.../main/java/org/openhab/binding/mqtt/espmilighthub/internal/handler/EspMilightHubHandler.java Outdated Show resolved Hide resolved
.../main/java/org/openhab/binding/mqtt/espmilighthub/internal/handler/EspMilightHubHandler.java Outdated Show resolved Hide resolved
.../main/java/org/openhab/binding/mqtt/espmilighthub/internal/handler/EspMilightHubHandler.java Outdated Show resolved Hide resolved
.../main/java/org/openhab/binding/mqtt/espmilighthub/internal/handler/EspMilightHubHandler.java Outdated Show resolved Hide resolved
.../main/java/org/openhab/binding/mqtt/espmilighthub/internal/handler/EspMilightHubHandler.java Outdated Show resolved Hide resolved
.../main/java/org/openhab/binding/mqtt/espmilighthub/internal/handler/EspMilightHubHandler.java Outdated Show resolved Hide resolved
@Skinah Skinah added rebuild Triggers Jenkins PR build and removed rebuild Triggers Jenkins PR build labels Jan 20, 2021
@Skinah
Copy link
Contributor Author

Skinah commented Jan 20, 2021

Thank you to both @cpmeister and @fwolter for doing the review. Looks like all has been addressed except 1 question awaiting a response on. The footer.xml has been changed to add this binding.

@Skinah
Copy link
Contributor Author

Skinah commented Jan 21, 2021

All issues are addressed.

@wborn wborn added rebuild Triggers Jenkins PR build and removed rebuild Triggers Jenkins PR build labels Jan 22, 2021
@Skinah
Copy link
Contributor Author

Skinah commented Jan 23, 2021

All changes requested have been made.

Skinah and others added 6 commits January 26, 2021 11:39
@Skinah
espmilighthub inital
a203643 
Signed-off-by: Matthew Skinner <matt@pcmus.com>
@Skinah
Update codeowners
39d2902 
Signed-off-by: Matthew Skinner <matt@pcmus.com>
@Skinah @fwolter
Update bundles/org.openhab.binding.mqtt.espmilighthub/src/main/java/o…
7452830 
…rg/openhab/binding/mqtt/espmilighthub/internal/handler/EspMilightHubHandler.java


Signed-off-by: Matthew Skinner matt@pcmus.com

Co-authored-by: Fabian Wolter <github@fabian-wolter.de>
Signed-off-by: Matthew Skinner <matt@pcmus.com>
@Skinah @fwolter
Update bundles/org.openhab.binding.mqtt.espmilighthub/src/main/resour…
1cfea9d 
…ces/OH-INF/thing/thing-types.xml


Signed-off-by: Matthew Skinner matt@pcmus.com

Co-authored-by: Fabian Wolter <github@fabian-wolter.de>
Signed-off-by: Matthew Skinner <matt@pcmus.com>
@Skinah @fwolter
Update bundles/org.openhab.binding.mqtt.espmilighthub/README.md
721dca6 
Signed-off-by: Matthew Skinner matt@pcmus.com

Co-authored-by: Fabian Wolter <github@fabian-wolter.de>
Signed-off-by: Matthew Skinner <matt@pcmus.com>
@Skinah @fwolter
Update bundles/org.openhab.binding.mqtt.espmilighthub/src/main/resour…
39b1dcf 
…ces/OH-INF/thing/thing-types.xml


Signed-off-by: Matthew Skinner <matt@pcmus.com>

Co-authored-by: Fabian Wolter <github@fabian-wolter.de>
Signed-off-by: Matthew Skinner <matt@pcmus.com>
Skinah and others added 11 commits January 26, 2021 11:39
@Skinah
BIG_DECIMAL_100
4faee1d 
Signed-off-by: Matthew Skinner <matt@pcmus.com>
@Skinah
remove repeative casting to percentType
4d24628 
Signed-off-by: Matthew Skinner <matt@pcmus.com>
@Skinah
Swap warn to thing going offline for config issue.
9f7a345 
Signed-off-by: Matthew Skinner <matt@pcmus.com>
@Skinah
Add to footer.xml
3296ec6 
Signed-off-by: Matthew Skinner <matt@pcmus.com>
@Skinah
reuse localBridge
d694f91 
Signed-off-by: Matthew Skinner <matt@pcmus.com>
@Skinah @cpmeister
Update bundles/org.openhab.binding.mqtt.espmilighthub/src/main/java/o…
a5001dc 
…rg/openhab/binding/mqtt/espmilighthub/internal/discovery/EspMilightHubDiscoveryService.java


Signed-off-by: Matthew Skinner <matt@pcmus.com>

Co-authored-by: Connor Petty <mistercpp2000@gmail.com>
@Skinah
remove double up.
f550060 
Signed-off-by: Matthew Skinner <matt@pcmus.com>
@Skinah @cpmeister
Update bundles/org.openhab.binding.mqtt.espmilighthub/src/main/java/o…
5d6fb39 
…rg/openhab/binding/mqtt/espmilighthub/internal/handler/EspMilightHubHandler.java


Signed-off-by: Matthew Skinner <matt@pcmus.com>

Co-authored-by: Connor Petty <mistercpp2000@gmail.com>
@Skinah
Spotless fixes
988e0c7 
Signed-off-by: Matthew Skinner <matt@pcmus.com>
@Skinah
update copyright header.
fffd679 
Signed-off-by: Matthew Skinner <matt@pcmus.com>
@Skinah
Remove hardcoded bridge
f5ab7f8 
Signed-off-by: Matthew Skinner <matt@pcmus.com>
@Skinah
use mqtt constant
2b2be94 
Signed-off-by: Matthew Skinner <matt@pcmus.com>
@Skinah Skinah requested a review from davidgraeff as a code owner January 26, 2021 01:16
@Skinah
Copy link
Contributor Author

Skinah commented Jan 26, 2021

All changes requested have been made and passes 'mvn clean install' on my machine.

cpmeister
cpmeister requested changes Jan 26, 2021
View reviewed changes
Copy link
Contributor

@cpmeister cpmeister left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think everything looks good code-wise. I'd just like some minor tweaks to some of the wording in the readme.

bundles/org.openhab.binding.mqtt.espmilighthub/README.md Outdated Show resolved Hide resolved
bundles/org.openhab.binding.mqtt.espmilighthub/README.md Outdated Show resolved Hide resolved
bundles/org.openhab.binding.mqtt.espmilighthub/README.md Outdated Show resolved Hide resolved
bundles/org.openhab.binding.mqtt.espmilighthub/README.md Outdated Show resolved Hide resolved
Skinah and others added 4 commits January 29, 2021 23:21
@Skinah @cpmeister
Update bundles/org.openhab.binding.mqtt.espmilighthub/README.md
005a690 
Signed-off-by: Matthew Skinner <matt@pcmus.com>

Co-authored-by: Connor Petty <mistercpp2000@gmail.com>
@Skinah @cpmeister
Update bundles/org.openhab.binding.mqtt.espmilighthub/README.md
f8af84d 
Signed-off-by: Matthew Skinner <matt@pcmus.com>

Co-authored-by: Connor Petty <mistercpp2000@gmail.com>
@Skinah @cpmeister
Update bundles/org.openhab.binding.mqtt.espmilighthub/README.md
d540a6e 
Signed-off-by: Matthew Skinner <matt@pcmus.com>

Co-authored-by: Connor Petty <mistercpp2000@gmail.com>
@Skinah @cpmeister
Update bundles/org.openhab.binding.mqtt.espmilighthub/README.md
de424e5 
Signed-off-by: Matthew Skinner <matt@pcmus.com>

Co-authored-by: Connor Petty <mistercpp2000@gmail.com>
@cpmeister cpmeister merged commit b2bb917 into openhab:main Jan 29, 2021
@cpmeister cpmeister added this to the 3.1 milestone Jan 29, 2021
lsiepel added a commit to lsiepel/openhab-addons that referenced this pull request Feb 9, 2021
@lsiepel
Revert "[espmilighthub] Initial contribution (openhab#9218)"
212974f 
This reverts commit b2bb917.
lucacalcaterra pushed a commit to lucacalcaterra/openhab-addons that referenced this pull request Feb 23, 2021
@Skinah @fwolter @lucacalcaterra
[espmilighthub] Initial contribution (openhab#9218)
896e367 
* espmilighthub inital

Signed-off-by: Matthew Skinner <matt@pcmus.com>

Co-authored-by: Fabian Wolter <github@fabian-wolter.de>
Co-authored-by: Connor Petty <mistercpp2000+gitsignoff@gmail.com>
Signed-off-by: Luca Calcaterra <calcaterra.luca@gmail.com>
@fab33
Copy link

fab33 commented Feb 23, 2021

Can't find this binding on 3.1.0M1 (jfrog archive).

themillhousegroup pushed a commit to themillhousegroup/openhab2-addons that referenced this pull request May 10, 2021
@Skinah @fwolter @themillhousegroup
[espmilighthub] Initial contribution (openhab#9218)
a726405 
* espmilighthub inital

Signed-off-by: Matthew Skinner <matt@pcmus.com>

Co-authored-by: Fabian Wolter <github@fabian-wolter.de>
Co-authored-by: Connor Petty <mistercpp2000+gitsignoff@gmail.com>
Signed-off-by: John Marshall <john.marshall.au@gmail.com>
@Skinah Skinah deleted the espmilighthub branch July 18, 2021 13:25
thinkingstone pushed a commit to thinkingstone/openhab-addons that referenced this pull request Nov 7, 2021
@Skinah @fwolter @thinkingstone
[espmilighthub] Initial contribution (openhab#9218)
cee0cd4 
* espmilighthub inital

Signed-off-by: Matthew Skinner <matt@pcmus.com>

Co-authored-by: Fabian Wolter <github@fabian-wolter.de>
Co-authored-by: Connor Petty <mistercpp2000+gitsignoff@gmail.com>
marcfischerboschio pushed a commit to bosch-io/openhab-addons that referenced this pull request May 5, 2022
@Skinah @fwolter
[espmilighthub] Initial contribution (openhab#9218)
e7a86f0 
* espmilighthub inital

Signed-off-by: Matthew Skinner <matt@pcmus.com>

Co-authored-by: Fabian Wolter <github@fabian-wolter.de>
Co-authored-by: Connor Petty <mistercpp2000+gitsignoff@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fwolter fwolter fwolter approved these changes

@cpmeister cpmeister cpmeister approved these changes

@davidgraeff davidgraeff Awaiting requested review from davidgraeff

Assignees

@fwolter fwolter

Labels
cre Coordinated Review Effort new binding If someone has started to work on a binding. For a new binding PR.
Projects
None yet
Milestone
3.1
Development

Successfully merging this pull request may close these issues.
5 participants
@Skinah @fwolter @fab33 @cpmeister @wborn